-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Display of vendor
and product
Keys in DDF Store
#7794
Conversation
Hey @Zehir, thanks for your pull request! Tip Modified bundles can be downloaded here. DDB changesModified
ValidationTip Everything is fine ! 🕙 Updated for commit 65d5975 |
This comment was marked as resolved.
This comment was marked as resolved.
vendor
and product
Keys in DDF Store
This comment was marked as resolved.
This comment was marked as resolved.
OK this PR is now ready to merge |
@@ -43,7 +43,7 @@ | |||
{ | |||
"name": "attr/swversion", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the path fix goes into a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cant this will make a merge conflict or you merge that PR and I redo an other one after
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see..
Ok then leave it in this PR and just add a small mention of this in the PR description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the 3rd note ;
Note: Some DDFs, like those for Lidl, have been moved to their respective directories to maintain a clean and organized structure. They was previously in the Tuya directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know you've opened Pandora's box with that? 😂
#7640
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few small comments.
The main thing is the path fix which should have its own PR.
Thanks, I removed the spaces @manup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to check the small comments.
"vendor": "Develco Products", | ||
"product": "AQSZB-110 TVOC sensor", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason why you chop the vendor name here and add another vendor in the product name? That goes for all Develco devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a web search and for me Frient it more a series name than a vendor. Maybe I am wrong ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SwoopX can you please suggest an example which can be used to make these consistent for Develco devices?
"vendor": "Develco Products", | ||
"product": "AQSZB-110 TVOC sensor", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason why you chop the vendor name here and add another vendor in the product name? Either you go with "Develco Products" or "frient" as vendor 🙂
That goes for all frient devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I have kept the vendor per directory but the DDF from the develco
and frient
directory should be merged, they are identical.
(See Discord messages)
@manup what we do with this PR ? |
Before starting to merge the other device PRs I'd like to get this in to hopefully have less merge conflicts. Since we don't have a conclusion yet for the Develco DDFs, we have two options:
Note the PR is only about the user facing strings like they are shown in pages like: https://deconz-community.github.io/ddf-tools/#/store/search Imho we won't get a perfect solution with one PR but need to improve and align the strings over time. |
For me both is ok. But i would like to not wait more time to avoid more merge conflicts |
Well, I guess we can move ahead and pursue option 1 to get that thing out of the way 🙂 |
This pull request is now merged. The new DDB files have been uploaded to the store. DDB FilesModified
🕒 Updated for commit d648c8b |
Improve Display of
vendor
andproduct
Keys in DDF StoreThis PR aims to enhance the user experience by providing more meaningful values for the
vendor
andproduct
keys in the DDF store.Changes include:
Vendor Key: The value is now more reflective of the directory name, starting with a capital letter. In some cases, it matches the vendor name as found on the internet, providing a more recognizable reference for users. There are some exceptions to this rule, such as Xiaomi, which is split into Xiaomi Aqara and Xiaomi Mi to provide more specific vendor information.
Product Key: The product key now displays the name of the product in English, with the model ID appended in parentheses at the end. In cases where multiple model IDs exist, they are separated by a
/
.Note: Some DDFs, like those for Lidl, have been moved to their respective directories to maintain a clean and organized structure. They was previously in the Tuya directory.
These changes aim to make the DDF store more user-friendly and intuitive, by ensuring that key information is displayed in a clear and meaningful way.
Please review and provide any feedback.